Skip to content

Conversation

@MasterPtato
Copy link
Contributor

No description provided.

Copy link
Contributor Author

MasterPtato commented Jan 13, 2026

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more


How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Jan 13, 2026

More templates

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/cloudflare-workers@3879

@rivetkit/db

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/db@3879

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/framework-base@3879

@rivetkit/next-js

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/next-js@3879

@rivetkit/react

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/react@3879

rivetkit

pnpm add https://pkg.pr.new/rivet-dev/rivet/rivetkit@3879

@rivetkit/sql-loader

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sql-loader@3879

@rivetkit/virtual-websocket

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/virtual-websocket@3879

@rivetkit/engine-runner

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner@3879

@rivetkit/engine-runner-protocol

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner-protocol@3879

commit: 7d75756

@claude
Copy link

claude bot commented Jan 13, 2026

PR Review: Gasoline Documentation

This PR adds comprehensive documentation for Gasoline's core concepts. Overall, this is a high-quality documentation contribution that will significantly help developers understand the durable execution engine.

✅ Strengths

  1. Excellent Structure: Breaking the old monolithic GASOLINE.md into focused documents (OVERVIEW, WORKFLOW_HISTORY, etc.) greatly improves discoverability and maintainability.

  2. Clear Explanations: The workflow history location system is complex, and the documentation does an excellent job explaining it with concrete examples.

  3. Practical Examples: The Activities section's bad composition example (OVERVIEW.md:43-53) effectively illustrates common pitfalls.

  4. Good Technical Accuracy: The code example in WORKFLOW_HISTORY.md accurately demonstrates version-based conditional execution.

📝 Minor Issues & Suggestions

Documentation Content

  1. Inconsistent Actor Terminology (OVERVIEW.md:6)

    • Line 6: "Workflows - Similar to the concept of actors."
    • The old version explicitly mentioned "(not Rivet Actors)" to avoid confusion with Rivet's actor system
    • Suggestion: Consider re-adding this clarification: "Similar to the concept of actors (not Rivet Actors)" to prevent confusion with Pegboard actors
  2. Missing Newline (OVERVIEW.md:86)

    • The file should end with a newline character per standard conventions
    • Most editors and formatters expect this
  3. Clarity Enhancement (WORKFLOW_HISTORY.md:7)

    • "Each coordinate is a set of ordinates"
    • Could be clearer: "Each coordinate consists of one or more ordinates"
  4. Grammar (WORKFLOW_HISTORY.md:105)

    • "Loops structure event history with 2 nested branches"
    • Minor: Could be "Loops structure event history with two nested branches" (spell out small numbers in prose)

Code Changes

  1. Comment Clarity (engine/packages/gasoline/src/db/kv/mod.rs:980)
    • Changed "interpreted as u128" to "by internal representation"
    • While "by internal representation" is more accurate, it's less specific
    • Suggestion: Consider "by internal representation (u128 byte ordering)" to retain specificity

TODOs

  1. Placeholder Files
    • DESIGN_GUIDE.md, GOTCHAS.md, and OPERATIONS.md are all TODO placeholders
    • Consider: Creating issues to track these documentation items, or removing them until ready to populate
    • Placeholder files in main can create confusion about completeness

🎯 Recommendations

High Priority:

  • Add back the "(not Rivet Actors)" clarification to avoid terminology confusion
  • Add trailing newline to OVERVIEW.md

Medium Priority:

  • Consider whether to keep or remove placeholder TODO files
  • Enhance the comment in mod.rs to be more specific about the ordering

Low Priority:

  • Minor grammar/clarity improvements in WORKFLOW_HISTORY.md

Summary

This is solid documentation work. The content is technically accurate, well-organized, and pedagogically effective. The main concerns are around maintaining consistency with terminology conventions (Rivet Actors) and deciding whether to include placeholder files.

The typo fix in HIBERNATING_WS.md is good.

Verdict: Approved with minor suggestions for consideration. The documentation significantly improves upon the original GASOLINE.md file.

- Separate the transactions into 2 activities

## Signals

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice how it explains a bit more of how it actually works. Might be good to say blatanlty how signals are the only form of durable/guaranteed communication and they’re consumption is guaranteed ( is it?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to say "only". Signal consumption is not guaranteed, the wf needs to listen to the signals to receive them


Signals can be sent to a workflow if you know its name and a subset of its tags.

Internally, it is more efficient to order signal tags in a manner of most unique to least unique:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this Gotchas worthy?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely

@MasterPtato MasterPtato force-pushed the 01-13-chore_gas_add_overview_and_history_docs branch 2 times, most recently from ea05c93 to c2f846e Compare January 14, 2026 02:05
@MasterPtato MasterPtato force-pushed the 01-13-fix_otel_enrich_http_traces branch 2 times, most recently from a35f0d9 to e88b75c Compare January 14, 2026 02:53
@MasterPtato MasterPtato force-pushed the 01-13-chore_gas_add_overview_and_history_docs branch 2 times, most recently from 0738934 to 8b91b37 Compare January 14, 2026 03:06
@MasterPtato MasterPtato force-pushed the 01-13-fix_otel_enrich_http_traces branch from e88b75c to 74129c7 Compare January 14, 2026 03:06
@MasterPtato MasterPtato force-pushed the 01-13-chore_gas_add_overview_and_history_docs branch from 8b91b37 to f65fde5 Compare January 14, 2026 19:47
@MasterPtato MasterPtato force-pushed the 01-13-fix_otel_enrich_http_traces branch from 74129c7 to c3c6740 Compare January 14, 2026 19:47
@MasterPtato MasterPtato force-pushed the 01-13-chore_gas_add_overview_and_history_docs branch from f65fde5 to ffb8072 Compare January 14, 2026 20:01
@MasterPtato MasterPtato force-pushed the 01-13-fix_otel_enrich_http_traces branch from c3c6740 to cdb8231 Compare January 14, 2026 20:01
@MasterPtato MasterPtato force-pushed the 01-13-chore_gas_add_overview_and_history_docs branch from ffb8072 to 654b4e9 Compare January 14, 2026 22:45
@MasterPtato MasterPtato force-pushed the 01-13-fix_otel_enrich_http_traces branch from cdb8231 to 1353291 Compare January 14, 2026 22:45
@MasterPtato MasterPtato force-pushed the 01-13-chore_gas_add_overview_and_history_docs branch from 654b4e9 to c023ef5 Compare January 14, 2026 22:47
@MasterPtato MasterPtato force-pushed the 01-13-fix_otel_enrich_http_traces branch 2 times, most recently from 921ccb3 to e8bbcf8 Compare January 14, 2026 22:52
@MasterPtato MasterPtato force-pushed the 01-13-chore_gas_add_overview_and_history_docs branch from c023ef5 to af232ee Compare January 14, 2026 22:52
@MasterPtato MasterPtato force-pushed the 01-13-fix_otel_enrich_http_traces branch from e8bbcf8 to d8a0135 Compare January 14, 2026 23:02
@MasterPtato MasterPtato force-pushed the 01-13-chore_gas_add_overview_and_history_docs branch from af232ee to 1c80e13 Compare January 14, 2026 23:02
@MasterPtato MasterPtato force-pushed the 01-13-fix_otel_enrich_http_traces branch from d8a0135 to 116ea58 Compare January 14, 2026 23:07
@MasterPtato MasterPtato force-pushed the 01-13-chore_gas_add_overview_and_history_docs branch from 1c80e13 to 5889ab3 Compare January 14, 2026 23:07
@MasterPtato MasterPtato force-pushed the 01-13-fix_otel_enrich_http_traces branch from 116ea58 to 708a38f Compare January 14, 2026 23:39
@MasterPtato MasterPtato force-pushed the 01-13-chore_gas_add_overview_and_history_docs branch from 5889ab3 to 7d75756 Compare January 14, 2026 23:39
@graphite-app
Copy link
Contributor

graphite-app bot commented Jan 14, 2026

Merge activity

  • Jan 14, 11:40 PM UTC: MasterPtato added this pull request to the Graphite merge queue.
  • Jan 14, 11:41 PM UTC: CI is running for this pull request on a draft pull request (#3908) due to your merge queue CI optimization settings.
  • Jan 14, 11:42 PM UTC: Merged by the Graphite merge queue via draft PR: #3908.

graphite-app bot pushed a commit that referenced this pull request Jan 14, 2026
@graphite-app graphite-app bot closed this Jan 14, 2026
@graphite-app graphite-app bot deleted the 01-13-chore_gas_add_overview_and_history_docs branch January 14, 2026 23:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants